-
-
Notifications
You must be signed in to change notification settings - Fork 13.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jupyter.lib: init #278315
base: master
Are you sure you want to change the base?
jupyter.lib: init #278315
Conversation
Introduce 2 functions to help with creating a jupyter environment: - mkKernelFromHaskellEnv - mkKernelFromPythonEnv
jupyter-all is not really something usable out of the box, mostly something used as a test. This PR moves it to jupyter.tests to reflect that.
... though I've let the reference accessible (for now) in jupyter-console. I hope to remove `jupyter-console.mkConsole` in a next PR after more discussions, possibly along with `withSingleKernel` ?
we have many kernels available and nixpkgs should be a good way to configure software as complex as jupyter. Right now our API is not straightforward and thus needs documenting. This is not the final doc, it's an initial PR to bootstrap the effort and refine the nixpkgs API for jupyter. future work: document how to wrap jupyter-notebook
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting started on this!
General impression: I'd be happier with this if the API looked more like the one we discussed in #269331. Not sure how you're planning to evolve this though. At the moment, it seems to have roughly the same warts as the current system, where you have to make a definitions
object and pass it to a Jupyter function. Not as ergonomic as the proposed API IMO.
I think what we want to do here is maintain backwards compatibility with the old way of doing things, i.e. jupyter.override { definitions = ...; }
. But now we add the new API, jupyter.withKernels (kernels: [...])
.
A few other random comments:
- As regards IHaskell specifically, I came up with nice flake support for GHC 9.0 through 9.8. The maintainer of IHaskell didn't want to keep them all in the main repo, but they're available here. I think it might make sense to pull these in to Nixpkgs with
builtins.getFlake
. We can even put kernelspecs over there and expose them in the flake, and then they can get tested in the other repo's CI. And they will have the logo images and stuff. - I'm not sure about the change to rename
jupyter-all
--it seems like an unrelated change to discuss in its own PR. I actually thinkjupyter-all
is still useful to users as a top-level attribute, whilejupyterLib
is actually not (assuming we move towards the new API like I'm suggesting).
Type: | ||
mkKernelFromHaskellEnv :: Derivation -> AttrSet | ||
*/ | ||
mkKernelFromHaskellEnv = ghcEnv: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect to have 1 file per kernel. And perhaps not constructing raw Jupyter kernelspecs like this, because then we have to worry about making sure they're all valid. IIRC there is currently some machinery in Nixpkgs for constructing these with some type safety.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect to have 1 file per kernel
ok
And perhaps not constructing raw Jupyter kernelspecs like this, because then we have to worry about making sure they're all valid. IIRC there is currently some machinery in Nixpkgs for constructing these with some type safety.
there is in nixos/modules/services/development/jupyter/./kernel-options.nix. It's possible to use those outside of the module but it's not trivial IMO. It can be added transparently later.
Let me try to emphasize a point about the abstraction I was hoping we can arrive at. The idea is to make a jupyter-kernels.python3.withPackages (ps: [ps.scipy ps.matplotlib]) # ==> yields a kernel.json with this Python environment
jupyter-kernels.ihaskell.withPackages (ps: [ps.aeson]) # ==> yields a kernel.json with this Haskell environment The idea is to make a common These kernels will be a good building block, because then you can assemble a (As well as |
@thomasjm I've invited you to https://matrix.to/#/#jupyter:nixos.org , I think it will be easier to discuss the API details there. Let us know if that works with you. I had in mind two ways to approach the API:
This PR sticks to 1. because it's the current paradigm.
I didn't want to change this without knowing first if "definition" was an official jupyter semantic ? If not we could change it, I have no idea. I have questions about
to provide jupyter frontends, we could reuse the traditional nixpkgs system:
builtins.getFlake is experimental so we can't use it but that's a detail. FYI: I have the merge rights on ihaskell (because I created the first flake IIRC, thanks for your latest update).
I disagree, it's a completely arbitrary confusing package. One would think it contains all kernels (which could make sense, but hard to maintain) but it just contain 2, and not the most popular if i may say so. there should be a getResourceGroup function, with which you can use the manually created group without having to manage it with pulumi.
I like |
It is not.
Those parameters would rarely change I think. I'd suggest designing it so it can be overridden, i.e. (jupyter-kernels.python3.withPackages (ps: [...])).override { displayName = "foo"; } and/or (jupyter-kernels.python3.override { displayName = "foo"; }).withPackages (ps: [...])
I'd suggesting just providing
You've lost me here. Are these comments meant for a different thread?
Those sorts of config are properties of the specific Jupyter tool you're using. To me, configuring those is an orthogonal concern. However, I do think we should make it clear which tool you're using. Probably by having |
Description of changes
This is the first PR of hopefully others trying to tackle issues I mentioned in #269331, i.e., improve nixpkgs API regarding jupyter usage and document its usage.
This doesn't do everything I have in mind because I dont want to do N breaking changes before realizing jupyter maintainers disagree with the approach.
My idea is to merge small PRs like this iteratively so we can discuss the changes gradually since we have time until the next release.
For instance the documentation doesn't explain how to create jupyter-notebooks envs but this will be fixed by next release.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.